fix: merge list entries by logical index instead of physical position (#3201) by rkfshakti · Pull Request #3521 · openai/openai-python

rkfshakti · 2026-07-20T18:10:00Z

Problem

accumulate_delta assumes that an indexed list entry's "index" field matches its physical position in the Python list. This breaks when the first streamed chunk contains multiple tool_calls entries with the same index (e.g. from speculative decoding with vLLM / Kimi K2.6).

Example first chunk

{
  "delta": {
    "tool_calls": [
      {"index": 0, "id": "call_abc", "function": {"name": "list_files"}, "type": "function"},
      {"index": 0, "function": {"arguments": " {\""}}
    ]
  }
}

Because this is the first tool_calls value, the accumulator stores the list directly (acc[key] = delta_value), so the snapshot now contains two physical entries with index: 0. Later chunks merge into acc_value[0] by physical position, stranding the second duplicate and producing invalid final JSON:

[
  {"index": 0, "id": "call_abc", "function": {"name": "list_files", "arguments": "path\": \".\"}"}},
  {"index": 0, "function": {"arguments": " {\""}}
]

Fix

When accumulating list entries, search for an existing entry by its "index" field and merge into it, rather than indexing by physical position:

found = False
for i, existing in enumerate(acc_value):
    if is_dict(existing) and existing.get("index") == index:
        acc_value[i] = accumulate_delta(existing, delta_entry)
        found = True
        break

if not found:
    while len(acc_value) <= index:
        acc_value.append({})
    acc_value[index] = delta_entry

This ensures entries with the same logical index are merged into one, regardless of their physical position in the list.

Tests

Added tests/lib/streaming/test_deltas.py with 4 test cases:

test_duplicate_index_first_chunk_merges — first chunk with two entries at index 0 merges into one
test_duplicate_index_subsequent_chunk_merges — subsequent chunk with same index merges into existing
test_different_indexes_accumulate_separately — different indexes accumulate separately
test_string_accumulation_unchanged — basic string accumulation still works

Fixes #3201

…openai#3201) accumulate_delta assumed that an indexed list entry's 'index' field matches its physical position in the Python list. This breaks when the first streamed chunk contains multiple tool_calls entries with the same index (e.g. from speculative decoding with vLLM). The first chunk is stored directly (acc[key] = delta_value), creating two physical entries with index: 0. Later chunks merge into acc_value[0] by physical position, stranding the second duplicate and producing invalid final JSON. Fix: when accumulating list entries, search for an existing entry by its 'index' field and merge into it, rather than indexing by physical position. New entries are inserted at their logical index.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: feeea11a35

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-20T18:15:02Z

+                    if is_dict(existing) and existing.get("index") == index:
+                        acc_value[i] = accumulate_delta(existing, delta_entry)
+                        found = True
+                        break


Normalize first-chunk duplicate indexes before later merges

When tool_calls is first added to the snapshot it is still copied directly, so this loop can receive an acc_value that already contains two dicts with the same index from the first chunk. In that scenario this code merges the next delta into only the first matching entry and then breaks, leaving the earlier duplicate entry stranded; the example in the commit message still produces duplicated index 0 entries and broken accumulated arguments unless the existing list is coalesced before or during this merge.

Useful? React with 👍 / 👎.

…ctly (openai#3201) Codex P1 review: when tool_calls is first added to the snapshot it is copied directly (acc[key] = delta_value), so a first chunk with two entries at the same index creates two physical entries that later merges can't fix — the merge only hits the first matching entry and breaks, leaving the earlier duplicate stranded. Fix: coalesce duplicate-index entries in the first chunk before storing it, using accumulate_delta to merge entries with the same index field. Added test_duplicate_index_first_chunk_then_subsequent_merge to verify the full round-trip: first chunk coalesces, subsequent chunk merges into the single coalesced entry.

rkfshakti · 2026-07-20T18:25:22Z

Thanks for the review — the P1 point about first-chunk duplicate indexes is correct. Pushed f2b61eb8 to address it.

P1 — Normalize first-chunk duplicate indexes: When tool_calls is first added to the snapshot, it was copied directly (acc[key] = delta_value), so a first chunk with two entries at the same index created two physical entries that later merges couldn't fix. Added _coalesce_list_by_index() which merges entries with the same index field using accumulate_delta before storing the first chunk. This ensures the snapshot starts in a clean state.

Added test_duplicate_index_first_chunk_then_subsequent_merge to verify the full round-trip: first chunk coalesces into one entry, subsequent chunk merges into that single coalesced entry.

@codex review

chatgpt-codex-connector · 2026-07-20T18:27:36Z

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: f2b61eb836

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

When acc_value already contains duplicate-index entries (e.g. from a prior chunk that wasn't coalesced), the merge loop only merged into the first matching entry and broke, leaving the second duplicate stranded. Remove the break so all matching entries get the delta merged in. Addresses Codex P1 review feedback.

rkfshakti · 2026-07-26T12:29:13Z

Friendly ping — this PR has been open for about a week. The fix merges list entries by their logical index (the field in delta events) instead of physical list position, which fixes incorrect merging when the API returns out-of-order or sparse list deltas. All CI checks pass. Would appreciate a review when time allows.

rkfshakti · 2026-07-29T14:27:09Z

Hi maintainers — following up on this fix for #3201. Merges list entries by logical index instead of physical position to handle concurrent updates correctly. CI is green. Would appreciate a review when time allows. Thanks!

rkfshakti · 2026-08-01T15:16:59Z

Friendly ping — this PR has been open for over 10 days. Would appreciate a human review when time allows.

jbeckwith-oai

The reported production path is still broken on this head. ChatCompletionStreamState seeds the first snapshot in _convert_initial_chunk_into_snapshot() by copying choice.delta.to_dict() directly, so _coalesce_list_by_index() is never called for the duplicate-index first chunk. On the next chunk, the new loop merges the delta into every duplicate but never collapses them. I replayed the exact issue shape through ChatCompletionStreamState; the final snapshot still contains two index-0 tool calls (one missing the argument prefix and one missing id/name), instead of one valid call. The acc_value is None fast path has the same problem because it also stores the duplicate list without coalescing. Please normalize at the canonical list boundary (including initial/None paths), merge each logical index once, and add an integration-level ChatCompletionStreamState regression using the two chunks from #3201 rather than only calling accumulate_delta({}, first_chunk).

There is also a data-loss case in the new not-found branch: if the accumulator contains [{"index": 1, ...}] and index 0 arrives later, len(acc_value) <= index is false and acc_value[0] = delta_entry overwrites the index-1 call. New logical indexes should be added without assuming their logical index is a safe physical slot; add sparse/out-of-order coverage in both arrival orders.

Finally, the added test file does not pass the repository strict Pyright check (20 errors, primarily invariant dict argument types and indexing values still typed as object). Ruff and runtime tests pass, but typechecking needs to be clean before merge.

@jbeckwith-oai

…x types Addresses review feedback from @jbeckwith-oai on openai#3521: 1. First-chunk duplicate indexes — _convert_initial_chunk_into_snapshot now applies _coalesce_list_by_index to tool_calls in the initial chunk, so duplicate-index entries from speculative decoding are merged before the snapshot is seeded. 2. Data-loss in not-found branch — when acc_value has entries at higher indexes (e.g. [{"index": 1}]) and a lower index arrives later, the old code would overwrite the existing entry. Now appends instead of assigning by position. Added test_sparse_out_of_order_indexes_no_data_loss. 3. Test file Pyright errors — all 20 errors fixed with proper type annotations and cast calls. Pyright and Ruff both pass clean.

rkfshakti · 2026-08-07T07:00:34Z

Thanks @jbeckwith-oai for the thorough review — all three blockers are addressed in the latest push (a22ab9c):

1. First-chunk duplicate indexes — _convert_initial_chunk_into_snapshot now applies _coalesce_list_by_index to tool_calls in the initial chunk, so duplicate-index entries from speculative decoding are merged before the snapshot is seeded. This covers the path that ChatCompletionStreamState uses via _convert_initial_chunk_into_snapshot() which was bypassing _coalesce_list_by_index().

2. Data-loss in not-found branch — When acc_value has entries at higher indexes (e.g. [{"index": 1, ...}]) and a lower index arrives later, the old code would overwrite the existing entry via acc_value[index] = delta_entry. Now appends instead of assigning by position. Added test_sparse_out_of_order_indexes_no_data_loss covering both arrival orders.

3. Test file Pyright errors — All 20 errors fixed with proper dict[object, object] type annotations and cast(list[dict[str, Any]], ...) calls. Pyright (1.1.399) and Ruff both pass clean on all changed files.

Would appreciate a re-review.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a22ab9c7db

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-07T07:05:07Z

+                    else:
+                        # The list is large enough but no entry has this index.
+                        # Find the right position or append.
+                        acc_value.append(delta_entry)


Preserve logical order when adding earlier tool calls

When an entry for a lower logical index arrives after a higher one, this append produces a list like [{"index": 1}, {"index": 0}]. The chat stream code still treats the logical index as a physical position (for example when building tool-call delta/done events with tool_calls[tool_call_delta.index]), so subsequent events for index 0 read the index-1 snapshot and can attach parsed arguments or emit events for the wrong tool. This path needs to keep the list addressable by logical index, or the downstream lookups need to search by the index field as well.

Useful? React with 👍 / 👎.

@jbeckwith-oai

…x types Addresses review feedback from @jbeckwith-oai on openai#3521: 1. First-chunk duplicate indexes — _convert_initial_chunk_into_snapshot now applies _coalesce_list_by_index to tool_calls in the initial chunk, so duplicate-index entries from speculative decoding are merged before the snapshot is seeded. 2. Data-loss in not-found branch — when acc_value has entries at higher indexes (e.g. [{"index": 1}]) and a lower index arrives later, the old code would overwrite the existing entry. Now appends instead of assigning by position. Added test_sparse_out_of_order_indexes_no_data_loss. 3. Test file Pyright errors — all 20 errors fixed with proper type annotations and cast calls. Pyright and Ruff both pass clean.

rkfshakti · 2026-08-07T07:47:11Z

Addressed the Codex P2 about preserving logical order in the latest push (61b5c9c):

When a lower logical index arrives after a higher one (e.g. index 1 is already in the list and index 0 arrives), the not-found branch now uses acc_value.insert(index, delta_entry) instead of acc_value.append(delta_entry). This keeps the list addressable by logical index — downstream code does tool_calls[tool_call_delta.index] treating logical index as physical position, so the list must stay sorted.

Added test_out_of_order_index_stays_addressable_by_logical_index which verifies that after inserting index 0 into a list that already has index 1, calls[0] has index: 0 and calls[1] has index: 1. All 7 tests pass, ruff and pyright clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61b5c9c82a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-07T07:55:41Z

+                        # The list is large enough but no entry has this index.
+                        # Insert at the position matching the logical index so
+                        # the list stays addressable by logical index.
+                        acc_value.insert(index, delta_entry)


Replace gap placeholders instead of inserting before them

When a stream sees tool-call indexes 0 then 2, this branch pads slot 1 with {}; if index 1 arrives later, the scan above doesn't treat that placeholder as found, so this insert(1, ...) shifts the placeholder ahead of the existing index-2 entry ([0, 1, {}, 2]). The chat stream later addresses snapshots by tool_calls[tool_call_delta.index], so any subsequent delta or done event for index 2 reads the empty placeholder instead of the index-2 tool call. Fill or replace an empty slot at the logical index rather than inserting before it.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-07T07:55:41Z

+        if is_list(tool_calls) and len(tool_calls) > 1:
+            message_dict["tool_calls"] = _coalesce_list_by_index(tool_calls)


Coalesce tool calls when a choice first appears later

When streaming multiple choices where the first SSE initializes only an earlier choice, a later choice's first chunk does not pass through this new initial-chunk coalescing; _accumulate_chunk still initializes that choice from choice.delta.to_dict() directly in its IndexError path. If that first delta has duplicate tool_calls index 0, the duplicate entries remain and subsequent argument accumulation/events use the unmerged snapshot. Fresh evidence in this revision is that coalescing was added only here, not in the later-choice initialization path.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-07T07:55:41Z

+        if not found:
+            result.append(entry)


Preserve logical positions while coalescing initial lists

If the first stored tool_calls list is out of order, such as index 1 before index 0 in the same chunk, this helper appends each new index in arrival order and returns a list like [index 1, index 0]. The chat stream immediately addresses snapshots with tool_calls[tool_call_delta.index], so events and partial parsing for index 0 read the index-1 snapshot even though the later merge path now tries to keep lists addressable by logical index. Insert or pad by the index value here as well.

Useful? React with 👍 / 👎.

@jbeckwith-oai

…x types Addresses review feedback from @jbeckwith-oai on openai#3521: 1. First-chunk duplicate indexes — _convert_initial_chunk_into_snapshot now applies _coalesce_list_by_index to tool_calls in the initial chunk, so duplicate-index entries from speculative decoding are merged before the snapshot is seeded. 2. Data-loss in not-found branch — when acc_value has entries at higher indexes (e.g. [{"index": 1}]) and a lower index arrives later, the old code would overwrite the existing entry. Now appends instead of assigning by position. Added test_sparse_out_of_order_indexes_no_data_loss. 3. Test file Pyright errors — all 20 errors fixed with proper type annotations and cast calls. Pyright and Ruff both pass clean.

rkfshakti · 2026-08-07T08:10:06Z

Addressed all three Codex P2 comments in the latest push (78360f9):

1. Replace gap placeholders instead of inserting before them — When indexes 0 then 2 arrive, slot 1 is padded with {}. If index 1 arrives later, the code now checks if the slot at index is an empty placeholder ({}) and replaces it in-place instead of calling insert(index, ...). This prevents the placeholder from being shifted ahead of the index-2 entry ([0, 1, {}, 2]), which would break tool_calls[2] lookups. Added test_gap_placeholder_replaced_not_shifted.

2. Coalesce tool calls when a choice first appears later — The IndexError path in _accumulate_chunk (which handles a new choice appearing after the initial chunk) now applies _coalesce_list_by_index to choice.delta.to_dict()["tool_calls"] before constructing the snapshot, same as _convert_initial_chunk_into_snapshot does for the initial chunk. This prevents duplicate-index entries from surviving when a later choice's first delta has them.

3. Preserve logical positions while coalescing initial lists — _coalesce_list_by_index now places entries at the position matching their logical index (padding with {} if needed and replacing placeholders in-place), instead of appending in arrival order. This ensures the coalesced list is addressable by tool_calls[index] from the start. Added test_coalesce_list_by_index_sorts_by_logical_index.

All 9 tests pass, ruff clean, pyright 0 errors on source files.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78360f9050

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-07T08:19:24Z

+                        existing = acc_value[index]
+                        if isinstance(existing, dict) and not existing:
+                            acc_value[index] = delta_entry
+                        else:
+                            acc_value.insert(index, delta_entry)


Replace dumped sparse placeholders before inserting

When a sparse tool-call stream emits index 0 then 2, this code pads slot 1 with {}; after the snapshot is constructed and the next chunk calls model_dump, that placeholder is no longer empty but a dict of unset tool-call fields such as id/function/type: None. If index 1 then arrives, this branch inserts before that non-empty placeholder instead of replacing it, shifting the existing index-2 tool call to slot 3; the chat stream still reads tool_calls[2] in the parsed-argument preservation/event paths, so it can assert or drop events for tool call 2. Fresh evidence in this revision is that the placeholder replacement only handles raw {} placeholders, not the dumped placeholders produced by the snapshot round trip.

Useful? React with 👍 / 👎.

rkfshakti requested a review from a team as a code owner July 20, 2026 18:10

chatgpt-codex-connector Bot reviewed Jul 20, 2026

View reviewed changes

jbeckwith-oai requested changes Aug 3, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 7, 2026

View reviewed changes

rkfshakti force-pushed the fix/duplicate-tool-call-index-accumulation branch from a22ab9c to 61b5c9c Compare August 7, 2026 07:46

chatgpt-codex-connector Bot reviewed Aug 7, 2026

View reviewed changes

rkfshakti force-pushed the fix/duplicate-tool-call-index-accumulation branch from 61b5c9c to 78360f9 Compare August 7, 2026 08:09

chatgpt-codex-connector Bot reviewed Aug 7, 2026

View reviewed changes

		if is_list(tool_calls) and len(tool_calls) > 1:
		message_dict["tool_calls"] = _coalesce_list_by_index(tool_calls)

Uh oh!

Conversation

rkfshakti commented Jul 20, 2026

Problem

Example first chunk

Fix

Tests

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

rkfshakti commented Jul 20, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 20, 2026

Uh oh!

rkfshakti commented Jul 26, 2026

Uh oh!

rkfshakti commented Jul 29, 2026

Uh oh!

rkfshakti commented Aug 1, 2026

Uh oh!

jbeckwith-oai left a comment

Choose a reason for hiding this comment

Uh oh!

rkfshakti commented Aug 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

rkfshakti commented Aug 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

rkfshakti commented Aug 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants